-
Notifications
You must be signed in to change notification settings - Fork 10.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(gatsby-source-wordpress): MediaItem.excludeFieldNames / auto exclude interface types that have no fields #37062
feat(gatsby-source-wordpress): MediaItem.excludeFieldNames / auto exclude interface types that have no fields #37062
Conversation
…ot File nodes if those are cached
…ng type excluding all of it's fields
@@ -1493,7 +1493,6 @@ Array [ | |||
Object { | |||
"fields": Array [ | |||
"avatar", | |||
"databaseId", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because this field was already excluded in the test site but due to a bug that this PR happens to fix it's now properly excluded in the schema.
"template", | ||
], | ||
"name": "WpNodeWithTemplate", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These snapshot changes are because I added options.type.MediaItem.excludeFieldNames: [
template]
in the test site gatsby-config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WpNodeWithTemplate
is now automatically excluded because one of the types that implement that interface (WpMediaItem
in this test site) has excluded the only field that interface defines: template
.
Any type that implements an interface must have every field in that interface. So to prevent errors, this type must also be excluded as a result of excluding MediaItem.template
.
// if this is a union type, make sure the union type has one or more member types, otherwise schema customization will throw an error | ||
(!!type.possibleTypes && !!type.possibleTypes.length) | ||
) | ||
const implementingTypes = getTypesThatImplementInterfaceType(type) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I abstracted all this code into getTypesThatImplementInterfaceType
because it needs to be run during query generation as well as schema customization, where before it was only needed during schema customization.
gatsbyNodeTypes, | ||
fieldAliases, | ||
fieldBlacklist, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transformFields
now gets these values internally to make it easier to use transformFields
in multiple places (which this PR is doing now)
return ( | ||
!interfaceTypeSettings.exclude && | ||
fieldOfTypeWasFetched(type) && | ||
fieldOfTypeWasFetched(interfaceType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interface types that were not fetched (added during query building) shouldn't appear in the schema, so this filters them out
@@ -84,7 +116,7 @@ export const typeIsASupportedScalar = type => { | |||
return supportedScalars.includes(findTypeName(type)) | |||
} | |||
|
|||
const typeSettingCache = {} | |||
const typeSettingCache = new Map() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to use a Map for caching since this function is called tons of times during schema customization and query building
* custom scalar types aren't imlemented currently. | ||
* @todo make this hookable so sub-plugins or plugin options can add custom scalar support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not going to do this so I removed the todo
parentInterfacesImplementingTypes: typesThatImplementInterface, | ||
}) | ||
|
||
if (shouldSkipInterfaceType && interfaceType.name !== `Node`) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always need the WpNode
type no matter what and it causes other problems if it's auto-excluded
const parentTypeNameSettings = getTypeSettingsByType(parentType) | ||
const parentTypeNodesFieldTypeNameSettings = getTypeSettingsByType( | ||
parentTypeNodesField?.type | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the proper way to look up plugin options type settings, not sure why I did it the other way before ;p
cachedMediaItemNodeIds.map(async nodeId => { | ||
const node = await helpers.getNode(nodeId) | ||
|
||
const parentNode = | ||
node?.internal?.type === `File` && node?.parent | ||
? helpers.getNode(node.parent) | ||
: null | ||
|
||
return parentNode || node | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this was a separate bug where we were returning the wrong kind of node since the code surrounding this expects MediaItem nodes, not File nodes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! The comments were really helpful.
This allows the
options.type.MediaItem.excludeFieldNames
to be able to exclude field names on the MediaItem type during sourcing.While fixing this I discovered interface types that have no fields due to an implementing type having every field on that interface excluded needed to be auto excluded so that this fix doesn't cause build errors.
I released a canary as
gatsby-source-wordpress@7.2.0-alpha-wordpress-exclude-fields-on-media-item.14